-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for new absence operator (#33) #34
Conversation
Hi @Janosch-x . Thanks for this and sorry about the delayed response. My initial thinking was this new "operator" is not really a "group" and thus had started investigating an alternative approach. However the fact that the new construct allows for any sub-expression to appear as a So, this is probably the best approach now and it looks good. Cheers and thanks again! |
On further review I noticed a couple of things:
|
Hi @ammar, thanks for the feedback. I also pondered whether it really is a group, ending up with the same conclusion as you did. To be sure I even asked the author, and he does see it is a group :)
This is indeed a mistake. Do you have a preferred solution? I think |
Or maybe remove |
@ammar In accordance with other token files I've removed the new type constant from the There is also a syntax file test and a parser test for it now. Lastly, to match the naming in Onigmo, I have changed the name from "absent" to "absence". |
@Janosch-x Sorry for not responding sooner. I recently had an accident and have an arm and a shoulder in cast. Hope to be able to work again soon |
@ammar I'm sorry to hear that. Please don't feel pressurised by my landslide of PRs. There is no urgency there at all, I had just been bored. Wishing you a good recovery! |
Hi @Janosch-x, thanks for looking into the classification of the operator and the updates. Again, I'm very sorry for the very late response. I think this looks good and I would like to merge it. Cheers! |
Included in release 0.4.4 |
No description provided.